Merged
Conversation
Currently this fails, because the `TestClient` doesn't actually stream responses :( A work-around might be to implement stopping (i.e. get it to raise an exception and close the stream), then check the response includes some frames.
It's now possible to tell MJPEGStream that the stream has stopped. This terminates the streaming response, and gets around the lack of streaming support in starlette's test client. With a bit of thought, this could potentially fix the longstanding issue that MJPEG Streams prevent a labthings server from restarting.
rwb27
force-pushed
the
test-mjpeg-stream-2
branch
from
d93a68a to
febdac5
Compare
julianstirling
approved these changes
Jul 4, 2025
Contributor
julianstirling
left a comment
julianstirling
left a comment
There was a problem hiding this comment.
This looks good. We should make a note to go back and do function by function unit testing at some point.
Am I correct in understanding that the StopAsyncIteration error and handling now means the server should exit gracefully when we run ofm restart with a browser open?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The
MJPEGStreamoutput was the single biggest bit of untested code. There is now a very basic test that makes sure it sends a response with at least one frame separator in it.In doing this, it was necessary to add code to cleanly close the stream. This has changed the signature of the
stopmethod, which may require calling code to be updated.The
TestClientwe use to test without serving on HTTP does not support streaming: this means we get all the frames at once, which limits exactly what can be tested. Adding the code to cleanly stop the stream is important, as otherwise the TestClient would hang.